Skip to content

Singleton comments#1230

Merged
Janther merged 3 commits intomainfrom
singleton-comments
Aug 30, 2025
Merged

Singleton comments#1230
Janther merged 3 commits intomainfrom
singleton-comments

Conversation

@Janther
Copy link
Copy Markdown
Member

@Janther Janther commented Aug 10, 2025

instead of gathering comments at each node and collecting them at the updateMetadata step, we store them in a single place and retrieve them at the end of the parsing step.

@Janther Janther requested a review from fvictorio August 10, 2025 08:08
@Janther Janther added the performance Results in smaller size, better memory usage, or faster execution label Aug 10, 2025
this.isEmpty =
this.arguments.items.length === 0 && // no arguments
!this.comments.some((comment) => isBlockComment(comment)); // no block comments
!ast.cst.children().some(({ node }) => isBlockComment(node)); // no block comments
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the only node that has a change that is not removing updateMetadata


// Because of comments being extracted like a russian doll, the order needs
// to be fixed at the end.
this.comments = this.comments.sort((a, b) => a.loc.start - b.loc.start);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments are handled at the slangSolidityParser level

Copy link
Copy Markdown
Member

@fvictorio fvictorio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-approving with a minor comment.

Comment thread src/slang-printers/print-comments.ts
@Janther Janther force-pushed the singleton-comments branch from 55eeba4 to eb411f3 Compare August 30, 2025 16:39
@Janther Janther merged commit 5ac8158 into main Aug 30, 2025
7 checks passed
@Janther Janther deleted the singleton-comments branch August 30, 2025 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Results in smaller size, better memory usage, or faster execution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants